fix: prevent browser screenshot memory accumulation #8830
Conversation
- Remove old screenshots from apiConversationHistory after each browser action - Simplify BrowserSessionRow to display only current state without pagination - Remove screenshot history and pagination controls This prevents memory issues where screenshots accumulated indefinitely, consuming significant memory and wasting tokens by sending all historical screenshots to the model on every request. Now only the latest screenshot is kept in memory (~100-200KB) instead of potentially MBs of accumulated screenshots.
dosubot
bot
added
size:L
Review SummaryI've reviewed this PR and identified 1 issue that needs to be addressed:
The cleanup implementation doesn't distinguish between browser screenshots and user-uploaded images. If a user uploads a webp or png image during a conversation, it will be incorrectly deleted during browser actions. Recommendation: Modify the filter to only remove images from
|
![ellipsis-dev[bot]](https://avatars.githubusercontent.com/in/64358?s=60&v=4){kind=small}
| for (let i = messages.length - 1; i >= 0; i--) { | ||
| const message = messages[i] | ||
| if (message.say === "browser_action_result" && message.text && message.text !== "") { | ||
| const resultData = JSON.parse(message.text) as BrowserActionResult |
There was a problem hiding this comment.
JSON.parse on message.text (for browser_action_result and browser_action) isn’t wrapped in try/catch. Wrapping in try/catch would prevent crashes if message.text is malformed.
This comment was generated because it violated a code review rule: irule_PTI8rjtnhwrWq6jS.
![roomote[bot]](https://avatars.githubusercontent.com/in/1546624?s=60&v=4){kind=small}
| // Clean up old browser screenshots from API conversation history to prevent memory accumulation | ||
| // Only keep the latest screenshot - old ones are no longer needed by the model | ||
| if (browserActionResult?.screenshot) { | ||
| const apiHistory = cline.apiConversationHistory | ||
| for (let i = apiHistory.length - 1; i >= 0; i--) { | ||
| const message = apiHistory[i] | ||
| if (Array.isArray(message.content)) { | ||
| // Filter out old screenshot image blocks | ||
| message.content = message.content.filter((block) => { | ||
| // Remove base64 image blocks (browser screenshots) | ||
| // Keep text blocks and other content | ||
| if ( | ||
| block.type === "image" && | ||
| "source" in block && | ||
| block.source.type === "base64" && | ||
| (block.source.media_type === "image/webp" || block.source.media_type === "image/png") | ||
| ) { | ||
| // This is likely an old browser screenshot - remove it | ||
| return false | ||
| } | ||
| return true | ||
| }) | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
The cleanup logic removes ALL base64 webp/png images from the entire conversation history without distinguishing between browser screenshots and user-provided images. If a user uploads an image in webp or png format during the conversation, it will be incorrectly deleted during browser actions. The filter should only remove images from browser_action_result messages (by checking the message structure) and preserve the latest browser screenshot while removing only older ones. User-provided images in other message types must be preserved.
hannesrudolph
added
the
Issue/PR - Triage
2 participants
Summary
This PR fixes a critical memory issue where browser screenshots were accumulating indefinitely, consuming significant memory and wasting tokens by sending all historical screenshots to the model on every request.
Changes
apiConversationHistoryafter each browser actionBrowserSessionRowto display only current state without paginationImpact
Testing